Conversation
✅ Deploy Preview for vitess ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
ajm188
left a comment
ajm188
left a comment
There was a problem hiding this comment.
this is looking good!! i like the structure you're setting up with the operators guide page
|
I'm going to pull out the vtadmin(-api) and vtadmin-web reference pages into separate PRs. The skeletal operator's guide in this branch be changing a bunch once single-component vtadmin lands (vitessio/vitess#10118), but the program reference makes sense to commit now. |
doeg
force-pushed
the
sarabee-operators-guide
branch
3 times, most recently
from
a0744be to
453c8dd
Compare
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
doeg
force-pushed
the
sarabee-operators-guide
branch
from
f570b46 to
8c6eaff
Compare
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
ajm188
left a comment
ajm188
left a comment
There was a problem hiding this comment.
this looks great to me! (and thank you for putting this together! ❤️), i found one issue i think needs to be addressed, and one other "just curious" question, but i'm happy to merge after that first thing is addressed
|
|
||
| ## Prerequisites | ||
|
|
||
| - Building `vtadmin-web` requires [node](https://nodejs.org/en/) >= v16.13.0. |
There was a problem hiding this comment.
there's technically an issue if you go to >= 17.0.0 (see https://github.com/vitessio/vitess/blob/main/web/vtadmin/README.md#prerequisites), which we might want to call out here
There was a problem hiding this comment.
As a side note, it's possible to specify a more precise node version in the package.json engine property.
For example:
"node": "^16.13.0"allows versions that does not increment the first non-zero portion of semver; e.g., it includes 16.13, 16.420, and 16.99; it does not include 16.12 or 17.x. (Probably best?)"node": "16.13.0"disallows any other version.
The version is enforced (albeit weakly + not exactly how you'd expect or want) by way of the .npmrc file and will fail npm install when run with an unspecified version. More on that here: https://docs.npmjs.com/cli/v8/using-npm/config#engine-strict
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
ajm188
left a comment
ajm188
left a comment
There was a problem hiding this comment.
looks fantastic! is there anything else you want to get in this PR, or should I (or you!) merge now?
|
@ajm188 I can merge. Thanks for taking another look! |
2 participants
Signed-off-by: Sara Bee 855595+doeg@users.noreply.github.com
Deploy previews:
This does not cover the changes proposed in vitessio/vitess#10543, nor single-component VTAdmin (which would simplify this a lot). Neither does it go into much detail about deployment specifics (Docker, k8s, etc.).